-
Notifications
You must be signed in to change notification settings - Fork 447
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
IAM role support #362
base: develop
Are you sure you want to change the base?
IAM role support #362
Conversation
29b5c78
to
56ad18e
Compare
56ad18e
to
482efe1
Compare
Apologies for not getting back to you on this sooner after getting the 1.3.1 release out the door. As discussed on #344, if we do include this functionality, it needs to be something that is actively enabled by the user rather than something that might take non-EC2 users by surprise. Also it would be preferable to avoid burning Amazon IP numbers and paths ( I've dusted off and committed to develop (see 1cc7d12) a I would accept a pull request that looked for a new key iam_role_url (we can workshop the key name further…) in the .aws/credentials file:
and, in the absence of other credentials, used the presence of that to trigger code to I'd rather the JSON was parsed using string functions rather than regexps (htslib currently doesn't |
To me, rather than conflating the With regards to burning in this IP address - this shouldn't be a problem as this URL has always been used for AWS metadata, and the I completely agree on the regex parsing though - this was a very dirty hack and a better solution could definitely be found. |
People in coffee shops do not have the liberty of trusting their local network. |
True - I guess my point here is that given that the user will always have to manually enable IAM support, is there a difference in potential security vulnerabilities between using a simple It results in the same behaviour, it's just one is easier to activate than the other. |
True, the explicit switch is what protects people with miscreants on their local networks. So the reason for specifying the URL in the switch is (1) somewhat tenuously, there may be a different equivalent for other S3-alike providers; (2) we are not going to burn Amazon-specific IP numbers and paths into HTSlib. I would prefer a .aws/credentials key to a random environment variable, as that is more self-contained. However I have not verified that other .aws/credentials parsing tools (notably Amazon's!) quietly accept keys that they do not recognise. But cf #346. |
Noted. How about using KSON (from klib) for JSON parsing? I'd be happy to cook up a new PR based on your develop branch (with |
I noticed #344 and thought I would get the ball rolling. Here's an implementation that will fall back to using an IAM role when no other credentials are found.